-
Notifications
You must be signed in to change notification settings - Fork 334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix incorrect parsing of names for custom csr registers #1176
fix incorrect parsing of names for custom csr registers #1176
Conversation
6280641
to
093b63f
Compare
this commit fixes a regression introduced in ba8c1ee. The regression was caused by removal of these lines: ``` - /* Register prefix: "csr_" or "custom_" */ - strcpy(name, reg_type); - name[strlen(reg_type)] = '_'; ``` causing all CSR names with custom names to be parsed as empty strings.
093b63f
to
109646c
Compare
@JanMatCodasip , @en-sc take a look please. This commit should fix a regression, please see commit description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you.
So it looks like there is no test in riscv-tests/debug
that would cover custom CSRs (or full-custom registers), is that correct?
@JanMatCodasip to my surprise there are tests that should catch this issue. I don't understand why these tests passed. The regression is definitely there. Looks like something is wrong with testing - I'm trying to figure things out. |
Ok, I figured things out. No, there are not such tests. To trigger the issue you have to have something like this in your configuration file:
That is you have to specify a name for the exposed register. We have no such constructs in the configuration files used in riscv-tests. I'll add those shortly (once this change is merged in) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
this commit fixes a regression introduced in
ba8c1ee.
The regression was caused by removal of these lines:
causing all CSR names with custom names to be parsed as empty strings.